Adds ARROW_STRIP Marker type #190
Conversation
|
Hey @tfoote, could I get you to take a look at this? The corresponding Rviz PR is ready for review |
There was a problem hiding this comment.
Overall this seems like a good addition. My first reaction is to suggest that it be targeted for https://github.com/ros2/common_interfaces not here. This is a bit of a feature enhancement going into a stable release. But this is small enough that I would consider making it an exception.
However I would like to see it reviewed and merged to the new development version: https://github.com/ros2/common_interfaces and then once that's reviewed backported here. That way anyone who starts using it here won't get caught flat footed when they want to move forward.
| @@ -12,6 +12,7 @@ uint8 POINTS=8 | |||
| uint8 TEXT_VIEW_FACING=9 | |||
| uint8 MESH_RESOURCE=10 | |||
| uint8 TRIANGLE_LIST=11 | |||
| uint8 ARROW_STRIP=12 | |||
There was a problem hiding this comment.
Would it make more sense to call this a DIRECTED_LINE_STRIP as that's an already determined thing and we're just adding direction.
There's a few places below that reference LINE_STRIP that should be augmented.
There was a problem hiding this comment.
The geometry shares some functionality with ARROW, most notably the appearance and semantics of the scale field. I think ARROW_STRIP makes this connection more explicit, and conveys the same information as DIRECTED_LINE_STRIP in fewer characters.
That makes sense. The reason for targeting ROS1 was simply that it's the platform we're currently using, though we're planning to migrate to ROS2 next month. I'd be happy to raise the PR in https://github.com/ros2/common_interfaces, although due to software restrictions on my work machine, I likely won't be able to work on a ROS2 version of the corresponding Rviz PR until we start the migration in April. I'll raise that PR now 👍 |
|
Closed in favour of ros2/common_interfaces#218 |
|
Thanks for jumping it over there. |
Adds a new
ARROW_STRIPMarkertype for rendering batches of arrows.Rviz Issue: ros-visualization/rviz#1785
Rviz PR: ros-visualization/rviz#1786